-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat(property-mapping): add support for configurable Guid format #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ncipollina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really want to be able to specify format on a per field basis.
| /// </item> | ||
| /// </list> | ||
| /// </remarks> | ||
| public string GuidFormat { get; set; } = "D"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔄 I don't like these being separate fields. I would rather have a single Format string field. Depending on what the type is, that would determine where/how the format is applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be applying these on the DynamoFieldAttribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the overall format for all items. I am adding per-field formats next.
ncipollina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ The requested change will be handled in a future PR.
- Added `GuidFormat` option to `MapperOptions` with a default format of "D". - Updated `TypeMappingStrategyResolver` to respect the Guid format specified in `MapperOptions`. - Enhanced `PropertyMappingCodeRenderer` to handle type-specific arguments, including the Guid format. - Updated `DynamoMapperAttribute` to expose the `GuidFormat` option and document valid formats.
- Added instructions to mention related issues in the PR body. - Clarified the use of `closes #<issue number>` for resolving issues. - Updated the PR template to include related issues or PRs.
9194756 to
dc05483
Compare
… setup - Standardized syntax for dictionary and list initializations in tests. - Added trailing commas for multi-line collections to improve diff readability. - Replaced `Assert.Equal` with `Assert.Single` where applicable for clarity. - Improved parameter naming in `GetGuid` method calls across tests. refactor(Guid-handling): refine methods for Guid parsing with customizable format - Removed redundant Guid parsing methods to avoid code duplication. - Introduced default "D" format for `Guid` and nullable `Guid` parsing methods. - Simplified method signatures to improve usability and consistency. - Updated XML documentation to clarify `format` defaults and valid format strings.
…rameters - Removed redundant `format:` named parameters in method calls across test snapshots. - Updated property mappings to use positional arguments where applicable. - Simplified type-specific argument handling by treating all arguments as positional in `PropertyMappingSpecBuilder`.
🚀 Pull Request
📋 Summary
This PR adds support for configurable Guid format strings in DynamoMapper, allowing developers to control how Guid values are serialized to and from DynamoDB. The default format remains "D" (hyphenated format), but users can now specify alternative formats like "N" (no hyphens), "B" (braces), or "P" (parentheses) via the
GuidFormatproperty on the[DynamoMapper]attribute.Key Changes:
GuidFormatproperty toMapperOptions(internal) andDynamoMapperAttribute(public API)TypeMappingStrategyResolverto pass format string when mapping Guid typesPropertyMappingCodeRendererto correctly position theout varparameter after type-specific arguments✅ Checklist
🧪 Related Issues or PRs
Part of Phase 1 implementation for property-level customization.
💬 Notes for Reviewers
The key technical challenge here was adjusting the parameter ordering in
PropertyMappingCodeRenderer.cs(lines 98-104). Previously, theout varparameter was always inserted at position 1, but with type-specific arguments like format strings, it needs to be inserted after those arguments. The fix calculates the correct position dynamically based onFromTypeSpecificArgs.Length.This change maintains backward compatibility since the default Guid format remains "D", matching the existing behavior.